Skip to content

ext/curl: post multiple similarly-named files with curl - #22740

Merged
arnaud-lb merged 1 commit into
php:masterfrom
Sjord:curl-post-multi-file
Oct 14, 2026
Merged

ext/curl: post multiple similarly-named files with curl#22740
arnaud-lb merged 1 commit into
php:masterfrom
Sjord:curl-post-multi-file

Conversation

@Sjord

@Sjord Sjord commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@Sjord
Sjord force-pushed the curl-post-multi-file branch 2 times, most recently from c8f8de7 to aece997 Compare July 15, 2026 12:44
@Sjord
Sjord marked this pull request as ready for review July 15, 2026 12:58
@Sjord
Sjord requested a review from adoy as a code owner July 15, 2026 12:58
@Sjord Sjord mentioned this pull request Jul 17, 2026
Closed
@Sjord

Sjord commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@derickr Could you take a look at this? When you made #8292, was there a reason that it didn't work for files, or was this incidental?

@Sjord Sjord changed the title Post multiple similarly-named files with curl ext/curl: post multiple similarly-named files with curl Jul 30, 2026
@Sjord

Sjord commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@cmb69 could you perhaps review this please?

Girgias
Girgias requested changes Aug 7, 2026
View reviewed changes

@Girgias Girgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixing part of the refactoring with the behavioural changes makes it hard to follow along.

Could you maybe first split build_mime_structure_from_hash into multiple functions dealing with the different types in a different PR, and then rebase this PR when that one landed?

@Sjord

Sjord commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

I agree that the changes view of this PR is a mess. I moved the inner loop from build_mime_structure_from_hash to its own function. There are hardly any other changes. Splitting this up into multilple pull requests is not going to make things clearer, I'm afraid.

Could you take a look at this PR with white-space changes ignored? I think that makes it easier to see what's going on.

@Sjord
Sjord requested a review from Girgias August 11, 2026 09:20
@Sjord

Sjord commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

@Girgias could you take another look at this?

@Sjord

Sjord commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@arnaud-lb Could you take a look at this?

arnaud-lb
arnaud-lb reviewed Oct 12, 2026
View reviewed changes
Comment thread ext/curl/interface.c
Comment thread ext/curl/interface.c Outdated
Comment thread ext/curl/interface.c Outdated
Comment thread ext/curl/interface.c Outdated
Comment thread ext/curl/tests/curl_postfields_error.phpt Outdated
Comment thread ext/curl/tests/curl_postfields_error.phpt Outdated
Comment thread ext/curl/tests/curl_postfiles_array.phpt Outdated
@Sjord
Sjord force-pushed the curl-post-multi-file branch from f01e914 to 6ffa446 Compare October 12, 2026 17:41
arnaud-lb
arnaud-lb approved these changes Oct 13, 2026
View reviewed changes

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Could you add a NEWS and UPGRADING entry? I will merge after that

@Sjord
ext/curl: post multiple similarly-named files
b2ef91a 
Make it possible to post multiple files with the same name using cURL.
It was already possible to post files, and it was possible to post
multiple strings with the same name. This moves things around to combine
these two features.

Closes php#15859
@Sjord
Sjord force-pushed the curl-post-multi-file branch from 6ffa446 to b2ef91a Compare October 13, 2026 09:02
@Sjord

Sjord commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

@arnaud-lb
arnaud-lb merged commit 2544d0b into php:master Oct 14, 2026
18 checks passed
@arnaud-lb

arnaud-lb commented Oct 14, 2026

Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arnaud-lb arnaud-lb arnaud-lb approved these changes
@adoy adoy Awaiting requested review from adoy adoy is a code owner
@Girgias Girgias Awaiting requested review from Girgias

Assignees

No one assigned

Labels

Extension: curl

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Sjord @arnaud-lb @Girgias